fix: restore secure clipping to prevent landmass being cut at map edges#1385
Merged
fix: restore secure clipping to prevent landmass being cut at map edges#1385
Conversation
✅ Deploy Preview for afmg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The f2fc427 refactor replaced window.polygonclip (which had a secure=1 mode) with lineclip.clipPolygon, removing the behavior that added each boundary-crossing intersection point three times. Without this, the curveBasisClosed B-spline arcs away from the map edges instead of following them, leaving a visible ocean band at the map boundary. Restore the secure parameter on clipPoly: when secure=1, boundary points are duplicated twice after standard clipping, forcing the B-spline to stay on the boundary. Re-expose the parameter on window.clipPoly for legacy JS callers. Agent-Logs-Url: https://github.com/Azgaar/Fantasy-Map-Generator/sessions/0b83756d-4055-4265-9258-c23b0a3681cd Co-authored-by: Azgaar <26469650+Azgaar@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix bug with landmass cut along the map edges
fix: restore secure clipping to prevent landmass being cut at map edges
Apr 22, 2026
Azgaar
approved these changes
Apr 22, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores the legacy “secure clipping” behavior so landmass feature paths rendered with D3 curveBasisClosed stay pinned to the map boundary instead of bowing inward and leaving ocean-colored gaps at the edges.
Changes:
- Added an optional
secureparameter toclipPolythat duplicates boundary points to enforce B-spline knot multiplicity at map edges. - Restored secure clipping for feature path rendering by calling
clipPoly(..., 1)infeaturePathRenderer. - Re-exposed
secureviawindow.clipPoly(points, secure?)for legacy JS callers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utils/index.ts | Updates the global window.clipPoly wrapper to accept an optional secure flag. |
| src/utils/commonUtils.ts | Implements secure handling in clipPoly and updates the Window typing accordingly. |
| src/renderers/draw-features.ts | Uses secure clipping for feature path rendering to avoid edge gaps. |
Comment on lines
+12
to
+44
| * @param secure - When truthy, duplicate boundary-crossing points to prevent B-spline | ||
| * curves from arcing away from map edges (restores original "secure clipping" behavior) | ||
| * @returns Clipped polygon points | ||
| */ | ||
| export const clipPoly = ( | ||
| points: [number, number][], | ||
| graphWidth: number, | ||
| graphHeight: number, | ||
| secure?: number, | ||
| ) => { | ||
| if (points.length < 2) return points; | ||
| if (points.some((point) => point === undefined)) { | ||
| window.ERROR && console.error("Undefined point in clipPoly", points); | ||
| return points; | ||
| } | ||
|
|
||
| return clipPolygon(points, [0, 0, graphWidth, graphHeight]); | ||
| const clipped = clipPolygon(points, [0, 0, graphWidth, graphHeight]); | ||
|
|
||
| if (!secure || !clipped.length) return clipped; | ||
|
|
||
| // Duplicate each boundary point twice so the B-spline passes through it | ||
| // rather than arcing away from the map edge (replicates polygonclip secure=1) | ||
| const secured: [number, number][] = []; | ||
| for (const point of clipped) { | ||
| secured.push(point); | ||
| if ( | ||
| point[0] === 0 || | ||
| point[0] === graphWidth || | ||
| point[1] === 0 || | ||
| point[1] === graphHeight | ||
| ) { | ||
| secured.push(point, point); | ||
| } |
There was a problem hiding this comment.
The implementation duplicates all points that lie on the map boundary, but the new JSDoc (and PR description) describe duplicating only boundary-crossing intersection points. If the intent is to match the original polygonclip secure=1 semantics more closely, consider detecting only the points introduced by clipping (or update the doc/PR description to reflect that all boundary points are triplicated).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
f2fc427resample-module refactor swappedwindow.polygonclip(custom, with asecuremode) forlineclip.clipPolygonand silently dropped thesecure=1call infeaturePathRenderer. Thesecuremode added each boundary-crossing intersection point 3× instead of 1×; without that multiplicity thecurveBasisClosedB-spline arcs away from the map edge rather than tracking it, leaving an ocean-coloured gap along any border the landmass touches.Before (seed=12):
After (seed=12):
src/utils/commonUtils.ts— Add optionalsecureparam toclipPoly. When set, each point that lands exactly on a map-boundary edge (x=0 | x=width | y=0 | y=height) is emitted 3× in the output, replicating the original B-spline knot-multiplicity behaviour that forces the curve to pass through the boundary.src/renderers/draw-features.ts— Restoresecure=1infeaturePathRenderer, matching the originalgetFeaturePathJS call.src/utils/index.ts— Re-exposesecureonwindow.clipPolyfor legacy JS callers.